Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unescape query parameter name #33401

Merged
merged 3 commits into from
Jun 18, 2021

Conversation

campersau
Copy link
Contributor

@campersau campersau commented Jun 9, 2021

PR Title
Unescape query parameter name in QueryHelpers

PR Description
Unescapes the query parameter name when there is no value. This is already done when there was a value.

There are currently no test in QueryHelpersTests for testing encoded keys / values, should I add some more?

Fixes #33394

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Jun 9, 2021
@Tratcher
Copy link
Member

Tratcher commented Jun 9, 2021

There are currently no test in QueryHelpersTests for testing encoded keys / values, should I add some more?

A little more test coverage would be appreciated.

@Tratcher Tratcher added this to the 6.0-preview6 milestone Jun 9, 2021
@Tratcher Tratcher self-assigned this Jun 9, 2021
@Tratcher
Copy link
Member

Tratcher commented Jun 9, 2021

Sorry, I just realized this conflicts with #32829 that re-wrote a lot of this code to be more efficient. This bug still exists in that new code, so we'll prioritize getting that PR merged and then rebase this on top. Hold tight.

@Tratcher Tratcher self-requested a review June 9, 2021 17:12
@Tratcher
Copy link
Member

Tratcher commented Jun 17, 2021

#32829 has been merged so this can now be rebased. Note the fix needs to go into the new code path #32829 added.

@campersau
Copy link
Contributor Author

@Tratcher updated the new code path and added tests for it.

@Tratcher Tratcher enabled auto-merge (squash) June 18, 2021 20:50
@Tratcher
Copy link
Member

Thanks for getting back to this.

@Tratcher Tratcher merged commit 43a46af into dotnet:main Jun 18, 2021
@ghost ghost modified the milestones: 6.0-preview6, 6.0-preview7 Jun 18, 2021
@campersau campersau deleted the unescapequeryparametername branch June 18, 2021 21:18
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QueryString key is not unescaped when value omitted
3 participants